Skip to content

Remove releases, introduce min version testing, match rust-bitcoin lint and fmt #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
tcharding merged 3 commits into from
Dec 18, 2025
Merged

Remove releases, introduce min version testing, match rust-bitcoin lint and fmt #60

tcharding merged 3 commits into from
Dec 18, 2025

Conversation

@nyonson
Copy link
Collaborator

@nyonson nyonson commented Dec 18, 2025 •
edited
Loading

Without the releases dependency graph, pretty easy to dogfood min and recent version testing. Also easier to adopt rust-bitcion's lint and format rules. I kept it a workspace even though it only has one package to dogfood the slightly more complex case. Plus, might add some packages in the future.

Closes #54

@nyonson nyonson changed the title Remove releases and introduce min version testing Remove releases, introduce min version testing, match rust-bitcoin lint and fmt Dec 18, 2025
tcharding
tcharding approved these changes Dec 18, 2025
View reviewed changes
Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 04f8b0a

@tcharding tcharding merged commit 3fe854d into rust-bitcoin:master Dec 18, 2025
2 checks passed
@apoelstra
Copy link
Member

apoelstra commented Dec 23, 2025

In e195b5a:

This fails to restore the existing lockfile if LockFile::Minimal.derive or LockFile::Recent.derive fail. We should use a drop guard here to ensure that restore_existing is run no matter how the function exits (or we should manually call restore_existing in each error path, which in this case might be fewer LOC).

@apoelstra
Copy link
Member

apoelstra commented Dec 23, 2025

We could change backup_existing to return a handle which calls restore_existing on Drop.

@apoelstra
Copy link
Member

apoelstra commented Dec 23, 2025

Otherwise 04f8b0a looks good. We should add avoid-breaking-exported-api = false to clippy.toml here because there is no public API we're exposing.

@nyonson nyonson mentioned this pull request Jan 2, 2026
Merged
nyonson added a commit that referenced this pull request Jan 13, 2026
@nyonson
Merge #62: Restore lock file on failure
cddf5e8 
9749597 Restore lock file on failure (Nick Johnson)

Pull request description:

  Follow up for the bug found by @apoelstra in #60 (comment). I thought the recommended drop guard pattern turned out pretty slick, if a little verbose.

ACKs for top commit:
  tcharding:
    ACK 9749597

Tree-SHA512: 330b2cdc87030c6ca5f42235df16c94502c3c06d7ab03959f777b25e20095b8e6283099b9d7a8651488eca634527d64e13452b5701b965abf644243e11768884
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tcharding tcharding tcharding approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

rargo-rbmt: Consider using clippy config from rust-bitcoin

3 participants

@nyonson @apoelstra @tcharding